Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/dynamic library rounding #106

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

clemlak
Copy link
Contributor

@clemlak clemlak commented Mar 29, 2024

@clemlak
Copy link
Contributor Author

clemlak commented Mar 29, 2024

Some tests are not passing anymore because of the implemented fix adding the remainder to the initial amount. Let's see how we can work this out.

This problem could lead to issues in NTokenG3M for example: if we update the weights the sum might be > 1e18 and transactions on that block will revert or the pool might be vulnerable.

@clemlak clemlak added 📃 contracts Anything related to the DFMM contracts (or strategies) 🔨 audit fix Patching audit findings labels Mar 29, 2024
@clemlak clemlak self-assigned this Mar 29, 2024
@clemlak clemlak requested review from Alexangelj and kinrezC March 29, 2024 14:43
@@ -22,7 +22,7 @@ function computeTradingFunction(
LogNormalParams memory params
) pure returns (int256) {
int256 a = Gaussian.ppf(int256(rX.divWadDown(L)));
int256 b = Gaussian.ppf(int256(rY.divWadDown(L.mulWadUp(params.mean))));
int256 b = Gaussian.ppf(int256(rY.divWadDown(L.mulWadDown(params.mean))));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to be in scope of this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it in all of the other ones as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(didn't mean to approve, misclick)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a "Git issue", I merged into main by mistake and reverted that commit but I think some branches are picking up the non-change as an actual change?

@kinrezC kinrezC self-requested a review March 29, 2024 19:20
@clemlak clemlak merged commit bf3c7bf into fix/spearbit-audit Apr 2, 2024
5 checks passed
@clemlak clemlak deleted the fix/dynamic-library-rounding branch April 2, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 audit fix Patching audit findings 📃 contracts Anything related to the DFMM contracts (or strategies)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants